feat(hooks): defineHook — typed hook authoring with ctx payload/wrap/abort - #6100
Draft
edusperoni wants to merge 16 commits into
Draft
feat(hooks): defineHook — typed hook authoring with ctx payload/wrap/abort#6100edusperoni wants to merge 16 commits into
edusperoni wants to merge 16 commits into
Conversation
Purpose-built container with an Angular-compatible surface (inject, runInInjectionContext, Injector, provide/provideLazy, forwardRef). Lookup is class-object first with a fallback to the decorator-set name, per injector level, so per-call string overrides and duplicated contract copies in the extensions tree resolve to the same provider. Includes a legacy provider kind that constructs Yok-style classes via annotate(), lazy side-effect loaders for path-based registration, transient retention, and reverse-instantiation-order disposal.
reportDeprecation() dedups per api+detail and logs at trace level by default; NS_DEPRECATIONS=warn|error previews the stricter stages so the same call sites can be escalated over releases. Wired at the external entry points only: param-name hook invocation, require-time extension registration, and dynamicCall help templating.
A class migrated off property injection has neither $hooksService nor $injector, so the decorator threw at hook-execution time. The global injector must stay last in the chain - tests stub the instance properties and rely on them winning.
Yok keeps its entire public surface, subclassability, and the global $injector, while storage and resolution delegate to the new container. Command routing, key commands, and the public-API builder are unchanged. Every legacy member now carries @deprecated JSDoc naming its replacement, mirrored on IInjector.
DoctorService and ProjectNameService become @contract abstract classes; their impls are renamed *Impl (externally invisible - outside resolution is by string name or token, never class identity). The subpath resolves through contracts/package.json rather than an exports map, so existing deep requires keep working, and the entry point is side-effect-free so a duplicated CLI copy in an extensions tree never boots a second runtime.
Fixtures exercise the surfaces third parties rely on: param-name hook signatures across every payload shape and influence channel (mutation, function-return middleware, abort), require-time extension registration against global.$injector including hierarchical commands, and the full IInjector facade surface. Notably they pin that a hook naming an unwrapped payload key (the after-watchAction shape) is skipped as invalid - resolution changes must not resurrect long-dead hooks.
dependency-injection.md covers tokens (@contract), inject()/Injector, provider kinds, resolution semantics, forwardRef, coexistence with the legacy $injector, and a legacy-to-new quick reference. extending-cli.md leads with the recommended hook pattern - inject() works directly in hook bodies because they run in an injection context, pinned by a new compat test - and demotes parameter-name injection to a labeled legacy section. String-token lookups are framed as the migration bridge, not a co-equal API. The hook deprecation tracer now flags only hooks that actually use param-name service injection; a hookArgs-only signature follows the recommended pattern and stays silent.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
createHierarchicalCommand registered the synthesized parent - and its execute path resolved commandsService and errors - through the module-level global injector instead of the instance it was called on, so a parent dispatcher leaked onto the global injector whenever a hierarchical command was registered on any other instance. The require-ordering guard accidentally depended on that leak: with the parent now landing on the same instance, it is exempted for synthesized parents (they exist because a child registered, not because requires ran out of order), and the else branch mirrors the existing default-command skip. Net effect: register-then-require orderings that used to throw 'Default commands should be required before child commands' now work; nothing that worked before changes.
edusperoni
force-pushed
the
feat/define-hook
branch
from
July 30, 2026 01:34
d596600 to
9a43a9e
Compare
Internal code no longer reads global.$injector: the two cycle-bound sites (the deprecation tracer's logger fallback and @hook's last-resort lookup) go through a getInjector() accessor required at call time. The global property remains write-only from the CLI's side - it exists as the published surface for extensions and hooks.
edusperoni
force-pushed
the
feat/define-hook
branch
from
July 30, 2026 01:38
9a43a9e to
8453493
Compare
The token container was reachable only through a getter on the concrete Yok class, so every crossing from facade-typed code into the new API needed an any cast - the migration's most important seam was invisible to the type system. IInjector now declares readonly di: Injector.
edusperoni
force-pushed
the
feat/define-hook
branch
from
July 30, 2026 01:43
8453493 to
d75fed7
Compare
Yok is now an Injector - class Yok extends Injector - so the new API works on the facade directly (get, register with Providers, createChild, runInInjectionContext($injector, ...)) and inject(Injector) inside legacy-constructed classes returns the facade itself instead of a second, inner container identity. The di bridge is gone. register() dispatches by argument shape: a string first argument is the legacy name-based form, anything else is a Provider. IInjector now extends the Injector class type, which constrains implementers to the real class hierarchy (Yok and its subclasses) - intentional, since the interface only ever described Yok.
IInjector recomposes from per-subsystem faces - CommandRegistry, KeyCommandRegistry, ModuleRegistry, PublicApiBuilder - each an @contract token the facade registers itself under. One object still implements everything until the subsystems are physically extracted; extraction then becomes a provider swap for the face's token instead of a consumer migration. Consumers can depend on the narrow face they actually use, and deprecation becomes per-face instead of a flat everything-is-legacy. The contracts are internal (lib/common/contracts) and deliberately not re-exported from nativescript/contracts; promoting one is a per-contract decision.
- doctor-service: printWarnings accepts an optional trackResult matching its contracts; runSetupScript returns the setup script result instead of resolving undefined; canExecuteLocalBuild no longer dereferences its optional argument - deprecation tracer: a report dropped for lack of a logger is no longer latched as delivered - it reports once a logger exists - yok: global.$injector is an accessor pair, so a direct third-party assignment stays synchronized with the binding getInjector() reads - docs: note the useValue + shared:false quirk; show where Injector is imported from in the late-lookup example
edusperoni
force-pushed
the
feat/define-hook
branch
from
July 30, 2026 02:25
d75fed7 to
514c272
Compare
optional resolves to null instead of throwing for unknown tokens (a found-but-misconfigured provider still throws); skipSelf starts at the parent, escaping a child scope's shadowing entry; self refuses parent fallthrough. self+skipSelf throws. host is deliberately absent - it is an Angular component-tree concept with no analog in this hierarchy. get()'s former second parameter - the legacy per-call ctorArguments bag - had exactly one caller, the facade's own resolve(name, bag). It moves to a protected getWithLegacyArguments channel, so the public get() aligns with Angular's shape today rather than after the legacy paths are deleted.
Hook authors can now export a definition built with defineHook instead
of a plain function whose shape the CLI has to infer. The handler takes
a context object with the operation payload, an explicit wrap() for
middleware around the hooked method, and abort() for stopping the hook
as either a failure or a warning.
Definitions are marked with Symbol.for("nativescript:cli:hookDefinition")
so a duplicated CLI copy in an extension's dependency tree still
recognizes them. The definition path skips parameter-name resolution,
the projectData promotion hack and the deprecation report; plain
function hooks keep running through the existing path unchanged.
lib/common/define-hook.ts is import-free so a hook can load it without
booting a second runtime, and it is re-exported from
nativescript/contracts.
Yok extends Injector on the base branch, so definitions run in runInInjectionContext(this.$injector, ...) directly and inject(Injector) inside a hook returns the facade itself.
edusperoni
force-pushed
the
feat/define-hook
branch
from
July 30, 2026 02:51
514c272 to
077c41b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
What is the current behavior?
Hooks are plain functions whose shape the CLI infers at runtime: services arrive via parameter-name injection (deprecated, runtime-traced), the payload via a magic
hookArgsparameter, middleware by returning a function (implicit and undocumented), and aborting by throwing an error carryingstopExecution/errorAsWarningfields.What is the new behavior?
A typed, explicit hook-authoring API — fully additive; every existing hook keeps working unchanged.
lib/common/define-hook.tsis import-free (loading it can never boot a second CLI runtime) and re-exported fromnativescript/contracts; definitions carry aSymbol.formarker so duplicated CLI copies in an extension tree recognize each other's definitions.projectDatapromotion hack, no deprecation report. Plain-function hooks are untouched..mjsdefault exports are recognized.wrap()middlewares feed the exact channel legacy returned-functions use, so they compose with the@hookdecorator chain identically (11 new tests pin payload identity/mutation, inject-in-handler, middleware wrap + short-circuit, abort both ways, legacy coexistence + tracer isolation,.mjs, and name-mismatch tracing).extending-cli.mdnow leads withdefineHook; plain-function hooks and parameter-name injection remain documented as the transitional and legacy tiers.Full suite: 110 files, 1609 passed / 38 skipped; the yok oracle, public-API test, and compat fixtures are untouched.